cli: use cached index with lib search (#1624) #1789
Conversation
There was a problem hiding this comment.
Thanks for the PR.
I see that you have taken the same patch for the core search and that's great!
I've made some comments to simplify this patch, IMHO after we validate these changes we can "backport" some of the suggestions to the core search command too.
Finally, the integration tests are failing, some of the tests expect the Upgrading library_index... message from the CLI, but this is no more happening after the patch. Could you fix the tests too? https://github.com/arduino/arduino-cli/runs/7155610048?check_suite_focus=true#step:10:6806
| // The behavior of now.After(T) is confusing if T < 0 and MTIME in the future, | ||
| // and is probably not what the user intended. Disallow negative T and inform | ||
| // the user that positive thresholds are expected. | ||
| if modTimeThreshold < 0 { | ||
| feedback.Error(tr("Timeout must be non-negative: %dns (%s)", modTimeThreshold, duration)) | ||
| os.Exit(errorcodes.ErrBadArgument) | ||
| } |
There was a problem hiding this comment.
I would not bother too much with validating the timeout since it's a hardcoded constant.
| // The behavior of now.After(T) is confusing if T < 0 and MTIME in the future, | |
| // and is probably not what the user intended. Disallow negative T and inform | |
| // the user that positive thresholds are expected. | |
| if modTimeThreshold < 0 { | |
| feedback.Error(tr("Timeout must be non-negative: %dns (%s)", modTimeThreshold, duration)) | |
| os.Exit(errorcodes.ErrBadArgument) | |
| } |
| // Valid duration units are "ns", "us" (or "µs"), "ms", "s", "m", "h". | ||
| // Use a duration of 0 to always update the index. | ||
| func indexNeedsUpdating(duration string) bool { |
There was a problem hiding this comment.
I'd like to change the indexUpdateInterval constant to a time.Duration like:
const indexUpdateInterval = 60 * time.Minuteso the indexNeedsUpdating function may take directly a time.Duration
| // Valid duration units are "ns", "us" (or "µs"), "ms", "s", "m", "h". | |
| // Use a duration of 0 to always update the index. | |
| func indexNeedsUpdating(duration string) bool { | |
| func indexNeedsUpdating(timeout time.Duration) bool { |
| feedback.Error(tr("Timeout must be non-negative: %dns (%s)", modTimeThreshold, duration)) | ||
| os.Exit(errorcodes.ErrBadArgument) | ||
| } | ||
| return modTimeThreshold == 0 || now.After(info.ModTime().Add(modTimeThreshold)) |
There was a problem hiding this comment.
Also this one can be much more simplified, something like
return time.Since(info.ModTime()) > timeout
should work.
|
I think that just removing these two lines Lines 456 to 457 in f556aff |
per1234
added
type: enhancement
3 participants
Please check if the PR fulfills these requirements
before creating one)
our contributing guidelines
UPGRADING.mdhas been updated with a migration guide (for breaking changes)Feature
A new copy of
library_index.jsonis downloaded every timelib searchis called (unconditionally).A new copy of
library_index.jsonis downloaded every timelib searchis called and either one of the following conditions exists:titled accordingly?
No
lib search#1624See how to contribute